Skip to content

feat: create shared contract test utilities package (SDK-1866)#1151

Closed
joker23 wants to merge 20 commits into
mainfrom
devin/SDK-1866-1772645970
Closed

feat: create shared contract test utilities package (SDK-1866)#1151
joker23 wants to merge 20 commits into
mainfrom
devin/SDK-1866-1772645970

Conversation

@joker23

@joker23 joker23 commented Mar 4, 2026

Copy link
Copy Markdown
Contributor

Requirements

  • I have added test coverage for new or changed functionality
  • I have followed the repository's pull request submission guidelines
  • I have validated my changes against all supported platform versions

Related issues

SDK-1866

Describe the solution you've provided

Creates @launchdarkly/js-contract-test-utils under packages/tooling/contract-test-utils/ to consolidate duplicated contract test code across all five JavaScript SDK contract test implementations.

New shared package contents:

  • types/CommandParams.ts & types/ConfigParams.ts — extracted from browser, imports updated to use minimal compat types
  • types/compat.ts — minimal LDContext, LDLogger, LDEvaluationReason types compatible with both client and server SDKs
  • types/ContractTestConfig.ts — typed configuration interface for the CLI config file
  • logging/makeLogger.ts — extracted from browser
  • client-side/TestHook.ts — moved from browser (fetch-based), imports from @launchdarkly/js-client-sdk-common for cross-SDK compatibility
  • client-side/TestHarnessWebSocket.ts — extracted from browser, parameterized with capabilities list, client entity factory, and optional connection change callback
  • server-side/TestHook.ts — moved from server-node (got-based)
  • server-side/ClientPool.ts — new generic ClientPool<T> replacing the duplicated Record<string, Entity> + counter pattern
  • adapter/startAdapter.ts — extracted REST↔WebSocket bridge from browser and react-native adapters into a shared startAdapter() function
  • bin/sdk-testharness-server.ts — CLI executable for running contract test tooling commands
  • bin/loadConfig.ts — config file loader supporting contract-test.config.{json,js,mjs,cjs,ts,mts}
  • README.md — developer guide covering CLI usage, config file format, subpath exports, and package architecture

CLI executable (sdk-testharness-server / sts):

The shared package now registers a CLI executable via the bin field in package.json, with two names:

  • sdk-testharness-server (full name)
  • sts (alias)

Currently supports one command:

  • sdk-testharness-server adapter — starts the REST↔WebSocket adapter server

Configuration is loaded from a contract-test.config.{json,js,mjs,cjs,ts,mts} file in the current working directory (similar to how Jest reads jest.config.*). If no config file is found, defaults are used. Example config:

{
  "adapter": {
    "wsPort": 8001,
    "httpPort": 8000
  }
}

Subpath export strategy:

  • "." — universal types, logging, ClientPool (source .ts for bundlers)
  • "./client" — client-side TestHook + TestHarnessWebSocket (source .ts for bundlers)
  • "./server" — server-side TestHook (compiled dist/ for Node.js runtime)
  • "./adapter" — REST-to-WebSocket adapter + ContractTestConfig type (compiled dist/ for Node.js runtime)

Migrations:

  • Browser: Removed local CommandParams.ts, ConfigParams.ts, makeLogger.ts, TestHook.ts. TestHarnessWebSocket now extends the shared base class, passing capabilities and newSdkClientEntity via constructor. ClientEntity.ts imports from @launchdarkly/js-contract-test-utils/client. Adapter package reduced to a minimal package.json that delegates to the sdk-testharness-server adapter CLI — no local source code or tsconfig needed.
  • Server-node: Removed local TestHook.ts, imports ServerSideTestHook from shared package.
  • Shopify-oxygen: ClientPool now wraps the shared generic ClientPool<LDClient> instead of managing its own Record + counter. Uses noExternal in tsup config to bundle the shared package inline.
  • Electron: Removed local CommandParams.ts, ConfigParams.ts, makeLogger.ts, TestHook.ts. ClientEntity.ts and ClientFactory.ts import types and utilities from shared package.
  • React Native: Removed local CommandParams.ts, ConfigParams.ts, makeLogger.ts, TestHook.ts, TestHarnessWebSocket.ts. App.tsx uses shared TestHarnessWebSocket directly with capabilities list and connection change callback. Metro config updated to support subpath exports and TypeScript .js extension convention. Adapter package reduced to a minimal package.json that delegates to the sdk-testharness-server adapter CLI — no local source code or tsconfig needed.

Documentation:

The shared package includes a comprehensive README.md covering:

  • CLI commands and usage (sdk-testharness-server / sts)
  • Config file format and supported extensions
  • Subpath exports table and import paths
  • Usage examples for adapter packages, client-side entities, and server-side entities
  • Build scripts reference
  • Package architecture overview

⚠️ Items for reviewer attention:

  1. Shopify Oxygen ClientPool ID format change: The original implementation returned IDs like "client-1", but the new generic ClientPool.nextId() returns just "1". This affects the Location header in responses (e.g., /clients/client-1/clients/1). Verify this doesn't break the test harness expectations.

  2. React Native Metro configuration: Added unstable_enablePackageExports flag to enable package.json exports field resolution, plus a custom resolveRequest that maps .js imports to .ts files. This handles TypeScript's ESM convention where import './index.js' resolves to ./index.ts. The unstable_ prefix suggests this Metro feature may change in future versions.

  3. TypeScript module resolution workaround: The shared package uses "moduleResolution": "node" with a paths mapping for got pointing to ../../../node_modules/got/dist/source. This is necessary because @launchdarkly/js-client-sdk-common doesn't work with node16 resolution, while got v14's exports map requires it. This workaround is fragile but functional.

  4. CLI bin resolution via yarn: The browser and react-native adapter packages no longer have their own TypeScript source — they just invoke sdk-testharness-server adapter in their start scripts, relying on yarn's automatic bin resolution. Verify this works correctly in the CI environment. The adapters were tested locally and successfully resolved the bin through yarn's PnP.

  5. Config file loading for TypeScript: The config loader uses dynamic import() for .ts/.mts files. This requires Node.js >= 22 with native TypeScript support or a loader like tsx. If the CI environment uses an older Node version without TypeScript support, .ts config files won't load (but .json, .js, .mjs, .cjs will work).

  6. No test coverage: This is a refactoring PR. Correctness relies on the existing contract test harness validating behavior. There are no unit tests for the shared utilities.

  7. Cross-SDK compatibility via js-client-sdk-common: The shared TestHook imports hook types from @launchdarkly/js-client-sdk-common instead of @launchdarkly/js-client-sdk to ensure compatibility across browser, electron, and react-native SDKs. This dependency provides the shared Hook interface all client SDKs implement.

Describe alternatives you've considered

  • Considered using native fetch for server-side TestHook instead of got, but kept got to minimize changes in this refactor.
  • Considered separate packages for client-side vs server-side utilities, but a single shared package is simpler for this scope.
  • Considered removing .js extensions from internal imports to avoid Metro resolution issues, but kept them to maintain Node.js ESM compatibility for server-side builds.
  • Considered having adapters continue compiling their own wrappers around startAdapter(), but simplified to direct CLI invocation for better ergonomics and less duplication.

Additional context

Link to Devin Session: https://app.devin.ai/sessions/83a6141bac50404e88e590a092884e19
Requested by: @joker23

This change consolidates all five JavaScript SDK contract test implementations into a single shared package, setting the foundation for maintainable contract test infrastructure across the monorepo. The final iteration adds a CLI executable (sdk-testharness-server / sts) with config file support (modeled after Jest's config pattern), further simplifying the adapter packages to just delegate to the shared CLI, and includes comprehensive documentation in the package README.


Note

Medium Risk
Medium risk because it refactors cross-SDK contract test harness infrastructure (adapter/CLI/WebSocket/hook plumbing) and changes Shopify Oxygen client ID formatting, which could break harness integration even though it's non-production code.

Overview
Adds a new internal workspace package, @launchdarkly/js-contract-test-utils, consolidating duplicated contract-test code (shared command/config types, logging, client/server test hooks, a generic ClientPool, and a reusable TestHarnessWebSocket) and introducing a sdk-testharness-server/sts CLI with configurable REST↔WebSocket adapter startup.

Migrates browser, React Native, Electron, server-node, and Shopify Oxygen contract test projects to depend on the shared package: browser/RN adapter packages become thin wrappers invoking the shared CLI, entities import shared types/hooks/WebSocket, server-node switches to shared server-side hook, Shopify Oxygen adopts the shared ClientPool (changing generated client IDs), and React Native updates metro.config.js to resolve package exports/subpath imports.

Written by Cursor Bugbot for commit 498810c. This will update automatically on new commits. Configure here.


Open with Devin

Create @launchdarkly/js-contract-test-utils package under packages/shared/contract-test-utils/ to eliminate code duplication across JavaScript SDK contract test implementations.

Changes:
- Extract CommandParams, ConfigParams, makeLogger into shared types
- Create client-side TestHook (fetch-based) and TestHarnessWebSocket
- Create server-side TestHook (got-based) and generic ClientPool<T>
- Migrate browser contract tests to use shared package
- Migrate server-node contract tests to use shared package
- Migrate shopify-oxygen contract tests to use shared package
- Register workspace in root package.json

Co-Authored-By: Steven Zhang <szhang@launchdarkly.com>
@joker23 joker23 requested a review from a team as a code owner March 4, 2026 17:54
@devin-ai-integration

Copy link
Copy Markdown
Contributor

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment and CI monitoring

@github-actions

github-actions Bot commented Mar 4, 2026

Copy link
Copy Markdown
Contributor

@launchdarkly/js-sdk-common size report
This is the brotli compressed size of the ESM build.
Compressed size: 25566 bytes
Compressed size limit: 26000
Uncompressed size: 125383 bytes

@github-actions

github-actions Bot commented Mar 4, 2026

Copy link
Copy Markdown
Contributor

@launchdarkly/js-client-sdk size report
This is the brotli compressed size of the ESM build.
Compressed size: 24212 bytes
Compressed size limit: 25000
Uncompressed size: 83755 bytes

@github-actions

github-actions Bot commented Mar 4, 2026

Copy link
Copy Markdown
Contributor

@launchdarkly/browser size report
This is the brotli compressed size of the ESM build.
Compressed size: 172130 bytes
Compressed size limit: 200000
Uncompressed size: 800872 bytes

@github-actions

github-actions Bot commented Mar 4, 2026

Copy link
Copy Markdown
Contributor

@launchdarkly/js-client-sdk-common size report
This is the brotli compressed size of the ESM build.
Compressed size: 21281 bytes
Compressed size limit: 24000
Uncompressed size: 110213 bytes

cursor[bot]

This comment was marked as resolved.

- Change package.json main/types/exports to point to src/index.ts instead of dist/
  so consumers can resolve the package without a separate build step
- Change imports from @launchdarkly/js-sdk-common to @launchdarkly/js-client-sdk
  which has proper exports for both bundler and node16 module resolution
- Remove @launchdarkly/js-sdk-common from dependencies (no longer needed directly)

Co-Authored-By: Steven Zhang <szhang@launchdarkly.com>
devin-ai-integration[bot]

This comment was marked as resolved.

cursor[bot]

This comment was marked as resolved.

- Create separate entry points: /client and /server subpaths
- Move ClientPool to universal exports (zero external dependencies)
- Browser imports from /client (no node-server-sdk dependency)
- Server-node imports from /server (includes got + node-server-sdk)
- Shopify-oxygen imports ClientPool from base path
- Add typesVersions for moduleResolution: node compatibility

Co-Authored-By: Steven Zhang <szhang@launchdarkly.com>
cursor[bot]

This comment was marked as resolved.

devin-ai-integration Bot and others added 3 commits March 4, 2026 18:20
… compat types

- Move package from packages/shared/contract-test-utils/ to packages/tooling/contract-test-utils/
- Replace SDK-specific type imports (LDContext, LDLogger, LDEvaluationReason) with local compat types
- Remove duplicate ClientPool export from server.ts
- Update root workspace entry

Co-Authored-By: Steven Zhang <szhang@launchdarkly.com>
- Update exports to point to compiled dist/ files for runtime compatibility
- Change tsconfig rootDir to 'src' for clean dist output
- Add shared package build step to all contract test build scripts
- Fixes ERR_MODULE_NOT_FOUND at runtime in server-node and shopify-oxygen

Co-Authored-By: Steven Zhang <szhang@launchdarkly.com>
- Client/base exports point to .ts source (Vite/tsup handle TS natively)
- Server export points to compiled dist/ (Node.js runtime needs .js)
- Add tsconfig.server.json to compile only server-relevant files
- Server-node build uses build:server to avoid compiling client code
- Browser/shopify-oxygen don't need shared package pre-built (bundlers handle TS)

Co-Authored-By: Steven Zhang <szhang@launchdarkly.com>
cursor[bot]

This comment was marked as resolved.

devin-ai-integration Bot and others added 5 commits March 4, 2026 18:45
Add noExternal for @launchdarkly/js-contract-test-utils so tsup bundles
the shared package code instead of keeping it as an external import.
This is needed because the base export points to .ts source files which
Node.js cannot execute at runtime.

Co-Authored-By: Steven Zhang <szhang@launchdarkly.com>
- Delete local CommandParams, ConfigParams, makeLogger, TestHook from electron contract tests
- Delete local CommandParams, ConfigParams, makeLogger, TestHook, TestHarnessWebSocket from react-native contract tests
- Update electron ClientEntity and ClientFactory to import from shared package
- Update react-native ClientEntity and App.tsx to import from shared package
- Change shared TestHook import from js-client-sdk to js-client-sdk-common for cross-SDK compatibility
- Add intentionalClose and onConnectionChange support to shared TestHarnessWebSocket
- Add SDKConfigProxyParams to shared ConfigParams for electron proxy support
- Add @launchdarkly/js-contract-test-utils dependency to both electron and react-native package.json

Co-Authored-By: Steven Zhang <szhang@launchdarkly.com>
…ntract tests

Co-Authored-By: Steven Zhang <szhang@launchdarkly.com>
…tion

Co-Authored-By: Steven Zhang <szhang@launchdarkly.com>
cursor[bot]

This comment was marked as resolved.

Co-Authored-By: Steven Zhang <szhang@launchdarkly.com>

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think it makes much sense to have this without the adaptor implementation (eg https://github.com/launchdarkly/js-core/tree/main/packages/sdk/browser/contract-tests/adapter) since the websocket contract is specific to that adaptor implementation. I think it would be good to have a shared adaptor implementation that is executable.

I am already trying to leverage the adaptor implementation from the browser in my react sdk branch (https://github.com/launchdarkly/js-core/tree/skz/react-sdk-next/packages/sdk/react/contract-tests).

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good call — the adapter was indeed duplicated identically between browser and react-native. I've now extracted it into the shared package as @launchdarkly/js-contract-test-utils/adapter (compiled to dist/ since it's Node.js code).

Both browser and react-native adapters are now one-liners:

import { startAdapter } from '@launchdarkly/js-contract-test-utils/adapter';
startAdapter();

The shared startAdapter() function accepts optional { wsPort, httpPort } config (defaults to 8001/8000). This should make it easy to reuse in the react SDK contract tests too.

See commit 5b4b3c1.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would be more effective if we made this an executable and read off modifications from a configuration file (like what jest does for example). Reference https://docs.npmjs.com/cli/v11/configuring-npm/package-json#bin and we can call the runner sdk-testharness-server (sts as an alias) and the startAdapter function could be called with the command sdk-testharness-server adapter.

I think the configuration file can be called contract-test.config and we should support different formats (eg json, js, ts, etc)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Implemented the CLI executable and config file support as suggested:

CLI (sdk-testharness-server / sts):

  • Registered via bin field in the shared package's package.json (npm bin docs)
  • sdk-testharness-server adapter calls startAdapter() with config from the config file
  • sts is registered as an alias

Config file (contract-test.config.*):

  • Supports .json, .js, .mjs, .cjs, .ts, .mts formats (similar to Jest's jest.config.*)
  • Loaded from the current working directory via loadConfig()
  • If no config file is found, defaults are used

Example contract-test.config.json:

{
  "adapter": {
    "wsPort": 8001,
    "httpPort": 8000
  }
}

Adapter packages simplified:
Both browser and react-native adapter packages no longer need their own source code or tsconfig — they just delegate to the CLI:

{
  "scripts": {
    "build": "yarn workspace @launchdarkly/js-contract-test-utils build:adapter",
    "start": "yarn build && sdk-testharness-server adapter"
  }
}

Exported ContractTestConfig type so consumers can type their config files:

import type { ContractTestConfig } from '@launchdarkly/js-contract-test-utils/adapter';

All 43 CI checks pass.

…-native contract tests

Co-Authored-By: Steven Zhang <szhang@launchdarkly.com>
cursor[bot]

This comment was marked as resolved.

Co-Authored-By: Steven Zhang <szhang@launchdarkly.com>
cursor[bot]

This comment was marked as resolved.

Co-Authored-By: Steven Zhang <szhang@launchdarkly.com>
devin-ai-integration[bot]

This comment was marked as resolved.

devin-ai-integration Bot and others added 4 commits March 5, 2026 17:34
Co-Authored-By: Steven Zhang <szhang@launchdarkly.com>
Co-Authored-By: Steven Zhang <szhang@launchdarkly.com>
…time

Co-Authored-By: Steven Zhang <szhang@launchdarkly.com>
…ile support

- Create sdk-testharness-server CLI with 'adapter' command
- Add contract-test.config.{json,js,mjs,cjs,ts,mts} config file support
- Register bin entries in shared package (sdk-testharness-server + sts alias)
- Simplify browser and react-native adapter packages to use CLI
- Remove adapter src/index.ts and tsconfig.json (no longer needed)
- Export ContractTestConfig type for typed configuration files

Co-Authored-By: Steven Zhang <szhang@launchdarkly.com>
devin-ai-integration[bot]

This comment was marked as resolved.

@joker23

joker23 commented Mar 6, 2026

Copy link
Copy Markdown
Contributor Author

The new package should have a README.md to guide developers on how to use it.

Co-Authored-By: Steven Zhang <szhang@launchdarkly.com>
@devin-ai-integration

Copy link
Copy Markdown
Contributor

@joker23 Added a README.md to packages/tooling/contract-test-utils/ covering:

  • CLI usage (sdk-testharness-server / sts) with commands and configuration
  • Config file format and supported extensions (.json, .js, .mjs, .cjs, .ts, .mts)
  • Subpath exports table (., ./client, ./server, ./adapter)
  • Usage examples for adapter packages, client-side entities, and server-side entities
  • Build scripts
  • Package architecture overview

@joker23 joker23 marked this pull request as draft March 7, 2026 05:01
@devin-ai-integration

Copy link
Copy Markdown
Contributor

Closing as requested — the changes will be broken into smaller incremental PRs.

devin-ai-integration Bot added a commit that referenced this pull request Mar 10, 2026
…1151

- Add src/types/compat.ts with minimal LDContext and LDEvaluationReason types
  compatible with both client and server SDKs (no SDK imports needed)
- Merge all types back into unified CommandParams.ts and ConfigParams.ts
  (includes both client-specific and server-specific fields)
- Delete src/client-side/types/ and src/server-side/types/ directories
  (no longer needed with compat types approach)
- Simplify barrel exports: client.ts re-exports from index, server.ts
  re-exports from types/ directly
- Remove @launchdarkly/js-server-sdk-common peer dependency
- Add necessary casts in server-node consumer for SDK-specific types

Co-Authored-By: Steven Zhang <szhang@launchdarkly.com>
joker23 added a commit that referenced this pull request Mar 13, 2026
… to shared utils [SDK-2009] (#1168)

**Requirements**

- [ ] I have added test coverage for new or changed functionality
- [ ] I have followed the repository's [pull request submission
guidelines](../blob/main/CONTRIBUTING.md#submitting-pull-requests)
- [ ] I have validated my changes against all supported platform
versions

**Related issues**

- [SDK-2009](https://launchdarkly.atlassian.net/browse/SDK-2009) — Add
server-side TestHook and ClientPool to contract-test-utils
- Subtask 2 of
[SDK-1866](https://launchdarkly.atlassian.net/browse/SDK-1866)

**Describe the solution you've provided**

Moves the server-node `TestHook` (which uses `got` for HTTP callbacks)
into the shared `@launchdarkly/js-contract-test-utils` package and
migrates both server-node and Shopify Oxygen contract tests to use
shared infrastructure. Also deduplicates shared types (command params,
config params) so both client and server contract tests consume them
from the shared package.

**Changes to `packages/tooling/contract-test-utils/`:**

- `src/server-side/TestHook.ts` — moved from server-node contract tests
(exact rename, no content changes)
- `src/server.ts` — barrel re-exporting from `./index.js` plus
`ServerSideTestHook` (mirrors `client.ts` pattern)
- `src/client.ts` / `src/index.ts` — barrel exports using `.js`
extensions (required for compiled ESM output)
- `src/types/compat.ts` — minimal `LDContext`, `LDEvaluationReason`, and
`LDLogger` type definitions compatible with both client and server SDKs,
allowing shared code to avoid importing from either SDK directly
- `src/types/CommandParams.ts` — imports from `compat.ts`; includes both
client and server command fields (e.g. `migrationVariation`,
`registerFlagChangeListener`)
- `src/types/ConfigParams.ts` — same pattern; includes both `clientSide`
and server-specific fields (`bigSegments`, `dataSystem`)
- `src/logging/makeLogger.ts` — now imports `LDLogger` from `compat.ts`
instead of `@launchdarkly/js-client-sdk-common`
- `package.json`:
- All three subpath exports (`.`, `./client`, `./server`) point to
compiled `dist/` output
- Three build scripts: `build` (universal only), `build:client` (client
+ universal), `build:server` (server + universal)
- `got` and `@launchdarkly/node-server-sdk` added as **optional peer
dependencies**

**Three-tier TypeScript build configuration:**

- `tsconfig.json` (base) — excludes **both** TestHook files and barrel
files (`client.ts`, `server.ts`). Compiles only universal code with zero
SDK dependencies. Used by `build` script.
- `tsconfig.client.json` — extends base, re-includes client files,
excludes server files. Requires `@launchdarkly/js-client-sdk-common`.
Used by `build:client` script.
- `tsconfig.server.json` — extends base, re-includes server files,
excludes client files. Requires `@launchdarkly/node-server-sdk` + `got`.
Used by `build:server` script.

**Migrations:**

- **server-node**: 
- Deleted local `TestHook.ts`; imports `ServerSideTestHook` from shared
package
- Replaced inline `clientCounter` + `Record<string, SdkClientEntity>` in
`index.ts` with shared `ClientPool<SdkClientEntity>`
- Imported `CommandParams`, `CreateInstanceParams`, `SDKConfigParams`
from shared package (~130 lines of duplicated local type definitions
removed)
- Added type casts where shared types use `unknown` or `string` but SDK
expects specific types (e.g., `pe.defaultValue as boolean`,
`migrationVariation.context as LDContext`,
`migrationVariation.defaultStage as LDMigrationStage`)
- **Shopify Oxygen**: 
- Replaced inline `Record<string, LDClient>` + counter with shared
`ClientPool<LDClient>`
  - Updated JSDoc comment to be more general (not Shopify-specific)
- Added `noExternal` to `tsup.config.ts` to bundle shared package inline
- Client IDs changed from `"client-1"` to `"1"` (auto-generated by
`ClientPool.add()`)

**CI Workflow Updates:**


- `.github/workflows/browser.yml` — added `build:client` step before
contract test adapter build
- `.github/workflows/electron.yaml` — added `build:client` step before
contract tests entity build
- `.github/workflows/react-native-contract-tests.yml` — added
`build:client` step before contract test adapter build
- `.github/workflows/react.yaml` — added `build:client` step **before**
topological build (must run before Next.js/Turbopack resolves imports)
- `.github/workflows/server-node.yml` — added `build:server` step before
contract test build
- `.github/workflows/shopify-oxygen.yml` — added `build` step before
contract test build

**Build command matrix by CI environment:**
- **Universal only** (`build`): shopify-oxygen — has no SDK dependencies
- **Client + universal** (`build:client`): browser, electron,
react-native, react — have `@launchdarkly/js-client-sdk-common` but not
`got`/`node-server-sdk`
- **Server + universal** (`build:server`): server-node — has
`@launchdarkly/node-server-sdk` + `got` but may lack client SDK

**Updates since last revision:**

Implemented three-tier TypeScript build configuration to handle
different SDK dependency profiles across CI environments:

1. **Split `tsconfig.json` into three configs**: Base config now
excludes both client and server TestHook files, making it universal-only
(zero SDK dependencies). New `tsconfig.client.json` and
`tsconfig.server.json` extend base and selectively re-include their
respective files.

2. **Added `build:client` script**: Browser/Electron/React-Native/React
CI workflows now explicitly run `build:client` to compile
`dist/client.js`, which was missing when they ran the default `build`
script (universal only).

3. **Shopify Oxygen uses universal build**: Changed from `build:server`
to `build` since Shopify Oxygen only imports from the base package (no
subpath), not `/server`.

4. **Fixed React workflow build ordering**: Moved `build:client` to run
**before** the topological `yarn workspaces foreach ... run build`
because the React contract tests' Next.js/Turbopack build happens during
that step and needs `dist/client.js` to already exist when resolving
`@launchdarkly/js-contract-test-utils/client` imports.

This fixes CI failures where different environments tried to compile
code that imported SDKs not installed in their focused dependency trees
(e.g., React CI trying to compile server-side TestHook which imports
`got`).

**⚠️ Items for reviewer attention:**

1. **Compat types pattern**: Types use minimal compatible definitions
(`LDContext = Record<string, any>`, `LDLogger = { error(...), warn(...),
... }`) to avoid importing from SDK packages directly. This allows one
unified set of type files to work for both client and server, but
requires consumers to add explicit casts when passing values to SDK
methods.

2. **Type safety tradeoff**: The casts in `sdkClientEntity.ts` could
mask real type errors if the contract test harness sends incorrect
types. However, this follows the pattern from PR #1151 which was already
merged and validated. Notable casts:
   - `pe.defaultValue as boolean | number | string` (3 locations)
   - `migrationVariation.context as LDContext` (1 location)
   - `migrationOperation.context as LDContext` (2 locations)
   - `params.identifyEvent!.context as LDContext` (1 location)
   - `migrationVariation.defaultStage as LDMigrationStage` (3 locations)

3. **Three-tsconfig approach**: Each CI environment must use its
appropriate build script. Using the wrong script causes "Cannot find
module" errors. Future files must be added to the appropriate tsconfig
exclusion lists.

4. **CI environment requirements**: 
- Browser/Electron/React-Native/React: use `build:client` (client deps
available, server deps missing)
- Server-node: use `build:server` (server deps available, client deps
may be missing)
   - Shopify-oxygen: use `build` (universal only, minimal deps)

5. **ClientPool API change**: `ClientPool.add(client)` now
auto-generates and returns the client ID instead of requiring an
explicit ID. Shopify Oxygen client IDs changed from `"client-1"` to
`"1"`. CI contract tests pass, confirming the test harness handles this
correctly.

6. **`got` path mapping**: `tsconfig.json` uses a hardcoded relative
path (`"../../../node_modules/got/dist/source"`) which is fragile and
depends on Yarn's hoisting layout. This is a workaround because
`@launchdarkly/js-client-sdk-common` doesn't work with `node16` module
resolution while `got` v14 requires it.

7. **React workflow build ordering is critical**: The `build:client`
step must run **before** the topological `yarn workspaces foreach ...
run build` because Next.js/Turbopack resolves the
`@launchdarkly/js-contract-test-utils/client` import during the contract
tests build. If `dist/client.js` doesn't exist yet, Turbopack fails with
"Module not found".


**Describe alternatives you've considered**

- **Three-tier type architecture** (universal/client/server split into
separate directories) — initially implemented but reverted in favor of
the simpler compat.ts pattern from PR #1151, which uses one unified set
of type files with minimal compatible type definitions
- **Asymmetric exports** (client from source, server from dist) —
attempted but couldn't make it work reliably across all CI environments;
unified dist/ approach with explicit build steps is more maintainable
- **Two-tsconfig approach** (just base + server) — didn't work because
client CI environments tried to compile universal files that imported
from `@launchdarkly/js-client-sdk-common`, causing peer dependency
warnings and potential issues. Three tiers (universal/client/server)
cleanly separates concerns.

**Additional context**


Link to Devin Session:
https://app.devin.ai/sessions/f0a2bd3c755e49c3b639bd3ee770effe
Requested by: @joker23

No test coverage added in this PR — the existing contract test suites
serve as integration tests for the shared utilities.

**CI Status:** ✅ All 41/41 checks passing


[SDK-2009]:
https://launchdarkly.atlassian.net/browse/SDK-2009?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ
[SDK-1866]:
https://launchdarkly.atlassian.net/browse/SDK-1866?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ
<!-- devin-review-badge-begin -->

---

<a href="https://app.devin.ai/review/launchdarkly/js-core/pull/1168"
target="_blank">
  <picture>
<source media="(prefers-color-scheme: dark)"
srcset="https://static.devin.ai/assets/gh-open-in-devin-review-dark.svg?v=1">
<img
src="https://static.devin.ai/assets/gh-open-in-devin-review-light.svg?v=1"
alt="Open with Devin">
  </picture>
</a>
<!-- devin-review-badge-end -->

<!-- CURSOR_SUMMARY -->
---

> [!NOTE]
> **Medium Risk**
> Moderate risk due to changes in contract-test build/export plumbing
and runtime wiring (new compiled `dist` exports, new `ClientPool` IDs,
moved hook implementation) that can break CI or contract test harness
interactions if misconfigured.
> 
> **Overview**
> Centralizes contract test shared code by turning
`@launchdarkly/js-contract-test-utils` into a compiled `dist/` package
with new `./client` and `./server` entrypoints, a shared `BaseTestHook`,
a new server-side `TestHook` (using `got`), and expanded shared
config/command types using SDK-agnostic *compat* types.
> 
> Migrates server-side contract test services (`server-node` and
`shopify-oxygen`) to use the shared `ClientPool` and shared types/hooks,
removing the server-node local `TestHook` and updating client ID
generation/handling.
> 
> Updates multiple CI workflows to explicitly build the appropriate
contract-test-utils variant (`build:client` or `build:server`) before
building/running contract tests, and adjusts Shopify Oxygen bundling to
include the shared utils.
> 
> <sup>Written by [Cursor
Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit
850f7d5. This will update automatically
on new commits. Configure
[here](https://cursor.com/dashboard?tab=bugbot).</sup>
<!-- /CURSOR_SUMMARY -->

---------

Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant